Skip to content

Allow passing --triple alone to select a Swift SDK#10022

Open
finagolfin wants to merge 1 commit into
swiftlang:mainfrom
finagolfin:triple
Open

Allow passing --triple alone to select a Swift SDK#10022
finagolfin wants to merge 1 commit into
swiftlang:mainfrom
finagolfin:triple

Conversation

@finagolfin
Copy link
Copy Markdown
Member

Simply aliases it to --swift-sdk triple and finally adds a help string for the flag, fixes #7330. I checked to make sure the --swift-sdk error strings still make sense and they do, because they use the generic word "query" anyway.

This passed all tests locally on macOS except for one:

Test Case '-[PackageModelTests.SwiftSDKBundleTests testTargetSDKDerivation]' started.
/Users/finagolfin/swift/swiftpm/Tests/PackageModelTests/SwiftSDKBundleTests.swift:537: error: -[PackageModelTests.SwiftSDKBundleTests testTargetSDKDerivation] : failed: caught error: "No Swift SDK found matching query `aarch64-unknown-linux-gnu` and host triple `arm64-apple-macosx14.0`. Use the `swift sdk list` command to see available Swift SDKs."
Test Case '-[PackageModelTests.SwiftSDKBundleTests testTargetSDKDerivation]' failed (0.121 seconds).

That test was added in #8034, in support of a strange config where you could use the host SDK but override the triple, ie ignoring the installed bundles. Supporting that is not going to work with selecting a triple from the installed bundles as requested in #7330, unless we check if no bundles are installed, and only then default to the host SDK override logic. That will be weird for the user, where the host SDK override is applied only if no bundles happen to be installed.

Instead, I think we should just throw out that host toolset override logic, and move this line to only apply to the now somewhat-deprecated destination config files.

This --triple flag was introduced in #2604 to really only apply to destination files at the time, and probably never worked well for the host destination it was also applied to then. It probably doesn't work very well for host SDK overrides today, unless you really know what you're doing.

Better to only apply this --triple flag to the old destination files, which it was first introduced for, and as a selector for Swift SDK bundles, as I do in this pull, throwing out the old host override logic and closing #8716 as unsupported. I think we have a free hand here, since this flag was never documented till now. I'll adjust that test and add a couple more depending on what's decided.

Let me know what you think, @jakepetroules, @kateinoigakukun, and @MaxDesiatov.

Simply aliases it to `--swift-sdk triple` and finally adds a help string
for the flag, fixes swiftlang#7330.
} else if let customCompileTriple {
(ID, swiftSDK) = try store.selectBundle(matching: customCompileTriple.tripleString, hostTriple: hostTriple)
} else {
fatalError("Neither an SDK nor a triple matched somehow.")
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should never hit, but just putting it in as a guard rail.

@kateinoigakukun
Copy link
Copy Markdown
Member

I'm not 100% confident that it doesn't break existing --triple usage, but I don't see any valid --triple usage except for the legacy --destination and Darwin triples, which should be handled by defaultSwiftSDK path. So I think it's relatively safe to land.

But specifying --triple alone works only when a user installs a single Swift SDK supporting the triple, so I think the recommended canonical form for specifying a specific SDK would remain --triple <triple> --swift-sdk <sdk-id>, so I'm worried that adding this support might confuse users.

@finagolfin
Copy link
Copy Markdown
Member Author

But specifying --triple alone works only when a user installs a single Swift SDK supporting the triple, so I think the recommended canonical form for specifying a specific SDK would remain --triple --swift-sdk , so I'm worried that adding this support might confuse users.

Yep, this pull merely implements a shorter form for when only a single SDK includes that triple, and if you happen to have multiple SDKs installed that contain the same triple, it now gives you a nice error about what to do, #9937. You can try that out for yourself with the current trunk snapshot toolchain by passing in --swift-sdk triple with multiple bundles installed, and this pull just aliases to that. 🙂

@jakepetroules
Copy link
Copy Markdown
Contributor

so I'm worried that adding this support might confuse users.

I think this is a reasonable argument -- it trains people to use something that may just break as soon as they install another SDK, which makes things feel unstable.

@finagolfin
Copy link
Copy Markdown
Member Author

I think this is a reasonable argument -- it trains people to use something that may just break as soon as they install another SDK, which makes things feel unstable.

Unfortunately, that is what all our doc does right now, just with the --swift-sdk <triple> flag atm, so this flag would change nothing in that regard.

Now that specifying both the SDK and the triple works with #9955, which we need to get into 6.3, #9998, I will update that doc to tell people to use both flags, then this could just remain a shortcut for those who know what they're doing. I'll try to get the compiler/SDK version checking in for 6.4, so that will take away the main source of problems, and allow people to use this flag alone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--triple option doesn't take installed Swift SDKs into account

3 participants